Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using Entity Framework Core with Blazor #10448

Closed
danroth27 opened this issue May 22, 2019 · 26 comments
Closed

Using Entity Framework Core with Blazor #10448

danroth27 opened this issue May 22, 2019 · 26 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@danroth27
Copy link
Member

We need to consider how EntityFramework Core should be used with Blazor:

  • DbContexts are typically setup as scoped services in ASP.NET Core, which means they live for a single request. With server-side Blazor apps the scope is the lifetime of the connection. What impact does this have on EF Core?
  • Blazor is a client-side UI framework. With other client-side UI frameworks, like WPF/WinForms, the guidance from the EF team is that the DbContext should be tied to the life of the current window. What's the equivalent concept in Blazor (client-side/server-side)?
  • Review Blazor + EF Core guidance with the EF team

@divega @ajcvickers

@danroth27 danroth27 added Needs: Design This issue requires design work before implementating. area-blazor Includes: Blazor, Razor Components labels May 22, 2019
@mkArtakMSFT mkArtakMSFT added this to the 3.0.0-preview8 milestone May 22, 2019
@detachmode
Copy link

yes, had this problem already and was wondering the same. Current solution for me was to create a new DB Context with the using statement inside every DataAccess method.

@vertonghenb
Copy link
Contributor

Current solution is to make it Transient for Server Side Blazor.

@divega divega changed the title Using EntityFramework Core with Blazor Using Entity Framework Core with Blazor May 22, 2019
@divega
Copy link
Contributor

divega commented May 22, 2019

Blazor is a client-side UI framework. With other client-side UI frameworks, like WPF/WinForms, the guidance from the EF team is that the DbContext should be tied to the life of the current window. What's the equivalent concept in Blazor (client-side/server-side)?

Agreed. My read is that the first thing we need is a pattern for creating DbContext instances in which the lifetime of the DbContext is decoupled from the request scope, but you still get all the DbContext configuration from DI.

(Note: I am not familiar with Blazor yet, but I am making a few assumptions, like that request scopes still exist, and that in the same application you can mix Blazor with regular "stateless Web" usages)

One thing that I believe @davidfowl mentioned to me is that we could register a DbContext factory service in DI to simplify creating DbContext instance in a more ad-hoc manner when a request scope cannot be relied upon to control its lifetime.

EDIT: Currently, you can do this by creating an instance with the new keyword, passing the DbContextOptions<TContext> resolved from DI to the constructor. But I believe there is value in a factory.

This would allow you to control the lifetime of each individual DbContext explicitly, associate it with other objects if that helps, etc. I assume it would also give you a bit more flexibility than just changing the lifetime on AddDbContext to Transient, especially if you have scenarios in which you need to create more than one instance of the same DbContext inside another object (e.g. multiple using inside a method).

@divega
Copy link
Contributor

divega commented May 22, 2019

If we identify a pattern equivalent to associating the lifetime of the context with a "window" that works in 80% of Blazor scenarios and we find a way to automate it, we can do that too.

@divega
Copy link
Contributor

divega commented May 22, 2019

BTW, I created dotnet/efcore#4441 more than three years ago to have a DbContextFactory<TContext> registered in DI, but we ended up deciding not to implement it because you can achieve the same by by creating the DbContext with the new keyword, passing the DbContextOptions<TContext> resolved from DI to the constructor.

I now believe again there is probably value in having the factory registered, but I am curious if anyone thinks the pattern above is sufficient.

@danroth27
Copy link
Member Author

yes, had this problem already and was wondering the same. Current solution for me was to create a new DB Context with the using statement inside every DataAccess method.

@detachmode @vertonghenb Part of this discussion is to ensure we've identified all of the related issues with using EF Core with Blazor today. To help us out, could you please clarify exactly what issues you hit?

@detachmode
Copy link

detachmode commented May 29, 2019

@danroth27
When registering the DbContext in the DI Container with AddDbContext this instance will be used over the lifetime of the request, which in case of Server-Side means, as long as the user stays on the page without manually triggering a reload (which in a SPA can be a long time). A DbContext will keep its loaded data from the DB cached, so if a table got updated in the DB (outside the current instance of the Blazor App) this data will not be visible to the user of the SPA, until he manually reloads the page. EF encourages to use a short lifetime for DBContext instances, so the caching of the data won't have this negative effect of stale data.

I went with the using statement over the transient solution, so I could use my data access layer even in Singleton instances, which was a design choice.

@mkArtakMSFT mkArtakMSFT added enhancement This issue represents an ask for new feature or an enhancement to an existing one cost: S and removed Needs: Design This issue requires design work before implementating. labels Jun 25, 2019
@mkArtakMSFT mkArtakMSFT added this to Triage in Blazor via automation Jun 25, 2019
@mkArtakMSFT mkArtakMSFT moved this from Triage to Preview9 in Blazor Jun 26, 2019
@novfal
Copy link

novfal commented Jul 22, 2019

I am using entity framework core but i have a issue, while fill data Value getting but not displaying when i am using include command/ after joining the related table how rectify that .

rynowak pushed a commit that referenced this issue Aug 1, 2019
Fixes: #5496
Fixes: #10448

This change adds a *utility* base class that encourages you to do the
right thing when you need to interact with a disposable scoped or
transient service.

This solution ties the lifetime of a DI scope and a service to a
component instance. Note that this is not recursive - we expect users to
pass services like this around (or as cascading values) if the design
dictates it.
rynowak pushed a commit that referenced this issue Aug 1, 2019
Fixes: #5496
Fixes: #10448

This change adds a *utility* base class that encourages you to do the
right thing when you need to interact with a disposable scoped or
transient service.

This solution ties the lifetime of a DI scope and a service to a
component instance. Note that this is not recursive - we expect users to
pass services like this around (or as cascading values) if the design
dictates it.
rynowak pushed a commit that referenced this issue Aug 1, 2019
Fixes: #5496
Fixes: #10448

This change adds a *utility* base class that encourages you to do the
right thing when you need to interact with a disposable scoped or
transient service.

This solution ties the lifetime of a DI scope and a service to a
component instance. Note that this is not recursive - we expect users to
pass services like this around (or as cascading values) if the design
dictates it.
@rynowak rynowak added the Done This issue has been fixed label Aug 1, 2019
@rynowak rynowak closed this as completed Aug 1, 2019
Blazor automation moved this from To do to Done Aug 1, 2019
@ryanelian
Copy link

ryanelian commented Sep 12, 2019

My DB Context was disposed when using Blazor:

System.ObjectDisposedException: Cannot access a disposed object. A common cause of this error is disposing a context that was resolved from dependency injection and then later trying to use the same context instance elsewhere in your application. This may occur if you are calling Dispose() on the context, or wrapping the context in a using statement. If you are using dependency injection, you should let the dependency injection container take care of disposing context instances.
Object name: 'MyDbContext'.
   at Microsoft.EntityFrameworkCore.DbContext.CheckDisposed()
   at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies()
   at Microsoft.EntityFrameworkCore.DbContext.Microsoft.EntityFrameworkCore.Internal.IDbContextDependencies.get_StateManager()
   at Microsoft.EntityFrameworkCore.Query.QueryContextDependencies.get_StateManager()
   at Microsoft.EntityFrameworkCore.Query.QueryContext.get_StateManager()
   at lambda_method(Closure , QueryContext , DbDataReader , ResultContext , Int32[] , ResultCoordinator )
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.AsyncQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
Exception thrown: 'System.ObjectDisposedException' in System.Private.CoreLib.dll

I am using:

            services.AddDbContextPool<MyDbContext>(options =>
            {
                var connectionString = Configuration.GetConnectionString("DB");
                options.UseNpgsql(connectionString);
            });

I also tried this to no avail:

            services.AddDbContext<MyDbContext>(options =>
            {
                var connectionString = Configuration.GetConnectionString("DB");
                options.UseNpgsql(connectionString);
            }, contextLifetime: ServiceLifetime.Transient);

How do I solve this problem?

@ADefWebserver
Copy link

ADefWebserver commented Sep 12, 2019

How do I solve this problem?

@ryanelian - Check your code that may wrap any usage (meaning queries) of MyDbContext that wraps the code in a "using" statement. The using statement causes the MyDbContext to be disposed of too early. Remove the using statements.

Instead, use code like this:

public class WeatherForecastService
    {
        private readonly MyDbContext _context;
        public WeatherForecastService(MyDbContext context)
        {
            _context = context;
        }
        public Task<List<WeatherForecast>>
            GetForecastAsync(string strCurrentUser)
        {
            List<WeatherForecast> colWeatherForcasts =
                new List<WeatherForecast>();
            // Get Weather Forecasts  
            colWeatherForcasts =
                (from weatherForecast in _context.WeatherForecast
                     // only get entries for the current logged in user
                     where weatherForecast.UserName == strCurrentUser
                 select weatherForecast).ToList();
            return Task.FromResult(colWeatherForcasts);
        }
    }

@ryanelian
Copy link

Hi thank you for replying.

I've just found the bug, it's NOT the using query.

It's because my Blazor <EditForm> uses Model property which is NULL initially:

<EditForm Model="Form" OnValidSubmit="SaveEdit">

    EditFormModel Form; // = new EditFormModel(); // initially this was NULL. I thought if I assign a value to it during `protected override async Task OnInitializedAsync()` it would work.

Unfortunately it exploded, and for some reason the exception message is the EF Core error message above. (huh)

@JeroMiya
Copy link

For those of you having DbContext entity tracking issues caused by DbContext instances that last longer than the particular Save/Delete/Update operation - the solution isn't to use OwningComponent because the DI scope lasts as long as the component does. What you want is a scope that lasts only as long as the operation, such that you throw away the DbContext between those operations, just as you would if using client side blazor and DbContexts scoped to particular HTTP requests.

For this, you'll need to inject IServiceScopeFactory directly, create a scope within your operation, and inject your DbContext via GetRequiredService from this temporary scope.

@dharmaturtle
Copy link

dharmaturtle commented Oct 21, 2019

To tag onto what @JeroMiya said, here's a class that I use to quickly create/dispose a DbContext.

public class DbExecutor {
    private readonly DbContextOptions<MyExampleDb> _options;

    public DbExecutor(DbContextOptions<MyExampleDb> options) =>
      _options = options;

    public void Command(Action<MyExampleDb> command) {
      using (var db = new MyExampleDb(_options)) {
        command(db);
      }
    }

    public T Query<T>(Func<MyExampleDb, T> query) {
      using (var db = new MyExampleDb(_options)) {
        return query(db);
      }
    }

    public async Task<T> QueryAsync<T>(Func<MyExampleDb, Task<T>> query) {
      using (var db = new MyExampleDb(_options)) {
        return await query(db);
      }
    }

  }

Usage:

var xx = await DbExecutor.QueryAsync(db => db.MyTable.Where(x => x.Name == myCriteria).ToListAsync());

@DrGriff
Copy link

DrGriff commented Nov 5, 2019

As per @danroth27 (first post in this thread), we need a robust pattern for how the EF should be used by Blazor (Server side). There have been multiple post on this thread, and I think we just need a bit of clarification on best practice from Microsoft.

The problem (as I see it) is that a Blazor (Server) App is a connected application and the Service (injected into the Page) is therefore long-lived. The page may have a very rich and responsive UI resulting in multiple calls to the Service (c.f multiple AJAX calls from a web page hitting the WebServer) at the same time. If the Service has a long-lived EF Context, then it means that multiple threads may call the Context at the same time, resulting in an InvalidOperationException (only one thread may access the Context at any one time).

In an above post, @detachmode said that a long running context caches data, and suggested that this may lead to a problem that users staying on the same page will end up seeing stale data. Is that true though? From the EF6 documentation (not found the equivalent for Core) it states:

If an entity with the same keys is already present EF will include it in the results of the query. Although EF will still issue the query against the database, this behavior can bypass much of the cost of materializing the entity multiple times.

My reading of this is that it will still go to the DB to see if the entity is different - if it is different, you'll get the latest version, but if it's the same, you'll get the previous version. Perhaps Microsoft can confirm...

If I'm interpreting this correctly then caching is advantageous and so you want a long lived Context (which would also remove the overhead of having to instantiate it every time). If so, then how do you prevent two threads from accessing the Context at any one time? One could use a SemaphoreSlim(1,1) to lock the context allowing just one thread at a time. But if that were the case then wouldn't Microsoft have built that into the EF Core framework?

Microsoft, what's the "best practice" for using EF a real-world, rich UI, Blazor-Server app?

@davidbaxterbrowne
Copy link

But if that were the case then wouldn't Microsoft have built that into the EF Core framework?

No. The threading has always happened at a higher level. WinForms and WPF have a single UI thread, and things like Forms and ViewModels that you can hang a long-lived DbContext from. ASP.NET has the Request scope, and ASP.NET guarantees that the a request-scoped DbContext is not simultaneously accessed from multiple threads.

@DrGriff
Copy link

DrGriff commented Nov 8, 2019

Seems like the preferred pattern for Blazor (Server) therefore looks to be:

services.AddDbContext<MyContext>(ServiceLifetime.Transient);
services.AddTransient<MyService>();

@danroth27
Copy link
Member Author

You can also derive from OwningComponentBase to create a DI scope for the component: https://docs.microsoft.com/aspnet/core/blazor/dependency-injection#utility-base-component-classes-to-manage-a-di-scope

@DrGriff
Copy link

DrGriff commented Nov 11, 2019

Great feedback. May I suggest that when a Developer chooses to create a new Blazor application in Visual Studio that the template is updated to put the "best practice" code for consuming the Entity Framework, even if it's commented out. This code should be "best practice" for a super-rich UI, not a Hello World app. If things work perfectly out-of-the-box then I think you'll get a higher adoption rate for Blazor.

@DrGriff
Copy link

DrGriff commented Nov 11, 2019

Having experimented with both approaches, I'm finding that the following seem to be identical in my app:

Razor Page
@inject MyService Service
Startup
services.AddDbContext<MyContext>(ServiceLifetime.Transient);
services.AddTransient<MyService>();

c.f.

Razor Page
@inherits OwningComponentBase<Data.MyService>
Startup
services.AddDbContext<MyContext>();
services.AddScoped<MyService>();

With both approaches, whenever a component is called, the constructor of both the Context and Service are called. However, I'm never seeing the Dispose() called on the Context nor the Service....thought the framework called this when it went out of scope.

Am I right then that this is an "either / or" approach, or is there a requirement to have:

Razor Page
@inherits OwningComponentBase<Data.MyService>
Startup
services.AddDbContext<MyContext>(ServiceLifetime.Transient);
services.AddTransient<MyService>();

@ctrl-alt-d
Copy link

ctrl-alt-d commented Nov 14, 2019

About OwningComponentBase + Transient

Current solution is to make it Transient for Server Side Blazor.

In my opinion OwningComponentBase and Transient is not enough. On blazor server side you can trigger two tasks from UI (almost) at a time. If service is not thread safe (and dbcontext is not), an exception will be raised.

I post an example, if you go to fetach data page and click for twice quickly on Load more data, an error is raised.

Peek 2019-11-14 16-40

Sample.

I post on s.zip a Minimal, Reproducible Example. It's blazor sample from templates with few changes.

Relevant code:

  • Startup.cs
        public void ConfigureServices(IServiceCollection services)
        {
            services.AddRazorPages();
            services.AddServerSideBlazor();
            services.AddTransient<WeatherForecastService>(); 
        }
  • WeatherForecastService
public bool InUse {get; private set;}= false;


public async Task<WeatherForecast[]> GetForecastAsync(DateTime startDate)
{
    if (InUse)
    {
        throw new Exception("Not thread safe object. Sorry");  //<-- fake dbcontext
    }
    lock(Summaries) InUse=true;
    var rng = new Random();
    await Task.Delay(1000);  //<-- fake delay
    var result = Enumerable.Range(1, 3).Select(index => new WeatherForecast
    {
        Date = startDate.AddDays(index),
        TemperatureC = rng.Next(-20, 55),
        Summary = Summaries[rng.Next(Summaries.Length)]
    }).ToArray();
    lock(Summaries) InUse = false;
    return result;
}
  • FetchData.razor
@page "/fetchdata"

@using s.Data
@inherits OwningComponentBase<WeatherForecastService> 

<h1>Weather forecast</h1>

<p>This component demonstrates fetching data from a service.</p>

<button class="btn btn-primary" @onclick="LoadData">Load More Data</button>

@if (forecasts == null)
{
    <p><em>Loading...</em></p>
}
else
{
    <table class="table">
        ...
    </table>
}

@code {
    List<WeatherForecast> forecasts ;

    protected override async Task OnInitializedAsync()
    {
        await LoadData();
    }

    protected async Task LoadData()
    {
        var data = await Service.GetForecastAsync(DateTime.Now);
        forecasts = forecasts ?? new List<WeatherForecast>();
        forecasts.AddRange( data );
    }

}

My current solution:

For me, at this time, the solution is to create a new scope for each "backend request".

@DrGriff
Copy link

DrGriff commented Nov 15, 2019

My original approach was to use a SemaphoreSlim(1,1,) to lock the context, but that presents problems:

  • You can't just lock it at the point it saves changes as another thread may be in the middle of making changes to entities, you'd have to lock it for the whole block of code where you're making changes to the context before the changes are persisted
  • Putting in locking is tantamount to adding a bottleneck

Have a read of this thread for Gustavo Navarro's Blazor.Grid. In this post he proposes controlling the lifetime of the Context (scroll down to his next entry if you want to see what problem he was experiencing). So the context is short-lived and has the scope of each operation (so, truly "transient").

What are the drawbacks to his approach?

  • Obviously, we're instantiating a new Context for each request, but if the Context is keps focused (lots of contexts, rather than one big one) then this shouldn't be an issue.
  • Are we losing any benefits of sharing a context? I'm afraid I'm not that familiar with the EF Core to be able to answer this just yet...

@dbrownems
Copy link

dbrownems commented Nov 15, 2019

Putting in locking is tantamount to adding a bottleneck

Yes. But a useful bottleneck. More of a resource policy that each client can only use one database connection at a time. It won't impact the aggregate throughput of your application.

Are we losing any benefits of sharing a context?

For a rich UI having a single, shared context for the duration of an operation will allow the user to make changes over a number of UI interactions and then call a single SaveChanges() for all the pending changes to be persisted in a single transaction, and with client-side optimistic concurrency.

@DrGriff
Copy link

DrGriff commented Nov 15, 2019

@dbrownems, @danroth27

Putting in locking is tantamount to adding a bottleneck

Yes. But a useful bottleneck. More of a resource policy that each client can only use one database connection at a time. It won't impact the aggregate throughput of your application.

Are we losing any benefits of sharing a context?

For a rich UI having a single, shared context for the duration of an operation will allow the user to make changes over a number of UI interactions and then call a single SaveChanges() for all the pending changes to be persisted in a single transaction, and with client-side optimistic concurrency.

What dbrownems says certainly sounds logical: the long-lived Context will be unique to that user (multiple people on the same Page will have different Contexts) so any locks can't affect other users.

So Microsoft, is locking the preferred way to go based on Dani Herrera's comments above?

As an FYI, this was my implementation (albeit cutdown)

public abstract class ServiceBase : IDisposable
{
      ~EciService()
      {
          this.Dispose(false);
      }

      protected SemaphoreSlim Padlock { get; } = new SemaphoreSlim(1, 1);
      public void Dispose()
      {
          this.Dispose(true);
          GC.SuppressFinalize(this);
      }
      protected virtual void Dispose(bool disposing)
      {
          this.ReleaseUnmanagedResources();
          if (disposing)
          {
              this.Padlock?.Dispose();
          }
      }
}

And the in Service

public class MyService : SerivceBase
{
      public MyService (MyContext context)
      {
          this.Context = context;
      }

      private MyContext Context { get; }

      public async Task<X> GetXAsync(...)
      {
            ...
            this.Padlock.Wait();
            try
            {
                  // All the code where I interact with the constructor injected Context.
            }
            ...
            finally
            {
                  this.Padlock.Release();
            }
      }
}

When I was using this, I had in the StartUp.cs

services
      .AddDbContext<MyContext>()
      .AddScoped<MyService>();

@DrGriff
Copy link

DrGriff commented Nov 18, 2019

@ctrl-alt-d (Dani Herrera)

I don't think that lock(Summaries) InUse=true; works in async code, you have to use a SemaphoreSlim(1,1)

@sven5
Copy link

sven5 commented Dec 10, 2019

Since this is a closed issue I don't think anyone is watching this....
However there are still plenty of threading issues. Currently I'm facing problems with Identity framework in Blazor because accessing the transient-registered UserManager often throws exceptions.

@danroth27
Copy link
Member Author

However there are still plenty of threading issues. Currently I'm facing problems with Identity framework in Blazor because accessing the transient-registered UserManager often throws exceptions.

@sven5 Could you please open a new issue for that and any other threading issues that you're hitting? Thanks!

@jaredpar jaredpar removed this from Done in Blazor Jan 7, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests